-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Attempt to fix the splash screen issue. #20157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Yes, not starting in another thread is for sure good! |
|
I built this PR on Windows 11. Dt crashes when I disable the splash screen. |
- Rename all routines from darktable_ prefix to dt_ to conform to style. - Never force the splash screen. We are then loosing the crawler messages but this could create a race condition. We create the splash in a different threads than the main one. This is wrong as the creation of the splash screen is not protected against concurrent access. Possibly fixes #19992 #20111
|
@gi-man : And you do not see very briefly the splash screen anymore, right? Are you willing to debug this with me? If I instrument the code, would you be able to compile, run and report what you see? |
Where? I only see it in |
It is removed from this PR. The creation was done on the main treads (my message was incorrect on this) but then multiple threads (if you include the crawler) where updating the splash with messages. |
|
Like I said, |
|
@dterrahe : Indeed, missed that, I really thought that the crawler was in an independent thread. That explain why this PR did not fix the issue. Back to debug... |
The screen showing up is on Fedora KDE current master. On Windows it initially just has the terminal, then dt window shows up with the tittle bar (the one with the X to close or minimize) with a white background. It will remain like this until I close/stop the process. I'm willing to debug. I am 6hrs behind you so there will be a delay. Yes, I can compile and run it on Windows. |
f7837cd to
53f7316
Compare
|
@gi-man : Perfect ! I have just push a new version I had prepared. The goal is to run darktable from command line with SPLASH_DEBUG set to some value and report if it work or not and the output in the console.
And we'll advise for the next step. Thanks for your help! |
|
Last but not least, do you have Lua scripts? If so, can you disable Lua completely (remove the luarc file). |
I have to drive into work today. I will continue to test in the evening. |
|
For clarity it did crash. I just did another run using -d all to see if it provides more insights. |
|
To run it should be on a standard Windows console: Sorry for not having been clear. |
version: darktable 5.5.0+97~g53f7316e96 darktable.exe --configdir C:\msys64\opt\test --cachedir C:\msys64\opt\test |
|
So not crash here, right? |
|
If no crash, now let's set SPLASH_DEBUG to other values and report if it crashes or not and the actual log. TIA. |
It did crash with SPLASH_DEBUG=15 |
|
As an experiment I tried, even with the show_splash_screen=TRUE, it crashes with any value in SPLASH_DEBUG!=0. If I set SPLASH_DEBUG=0 and show_splash_screen=TRUE, then no crash. |
This means that the issue is not really the splash screen as with value 15 the splash screen is a no-op. Only a printf to display some information and that's all. No dialog created, no widget created, no manual handling of events...
Ok, that's then basically the current code on master.
Even for value 2? as this only reset to NULL a widget that should be null anyway. |
|
show_splash_screen, SPLASH_DEBUG, Crash? |
|
What about 0,2 ? |
|
0, 4, Crash This one is different |
@gi-man Maybe you installed the wrong gdb package. A while back I also had a problem with gdb which was installed in /usr/bin from the Backtraces are an invaluable source of information in case of crashes, so it would be very helpful if you could run darktable under gdb. |
With this change on top of your PR. 0, 15, Crash 0, 1, Crash 0, 8, Crash 1, 0, No crash (just to confirm the build was good after the edits) I need to step away for a while. I will try gdb later tonight. |
|
I managed to get it to run in gdb. It does not crash. It is very slow. I tried it multiple times with and without the splash screen active. With the splash screen set to FALSE, the splash screen does show very briefly like on Fedora. I took a video with the cellphone. The bottom of the splash screen says: This is the only info in gdb: |
|
I wanted to rule out the UCRT packages updates, therefore I compiled 5.2.1 using the current UCRT. 5.2.1 does not crash with the splash screen off. |
|
@gi-man : Not sure what you call a crash. But if it crashes under gdb what we need is the backtrace. That is when you're back into gdb (you should have the prompt (gdb) ) then enter the command bt. |
I see, if you could find a combination where it crashes and get a backtrace it would be nice. Again, the fact that it does not crashes on gdb but crash on the same condition without gdb really favor a memory corruption or possibly a race condition. |
|
Something else to test is to build without Lua. I have found some comments in the code saying that we had issues... Be sure to clean completely the build directory:
And build without Lua:
I let you adapt the command for Windows. |
|
I'm using --clean-all in the build.sh Since it is not crashing in gdb (dt works but very slow), I decided to start bisection approach. It takes a while on windows to do the builds. I'm at this commit from 27Sep and it crashes here 1dbb8b6 We know it doesnt crash at 5.2.1. I'm at home today, so I should be able to find the commit. |
|
@gi-man : Thanks! It will be a very big progress. |
|
Let's bet on the changes in |
|
You are suggesting 1270eac could be improved by checking that the old view is not null (i.e. that this is not the initial change to lighttable, which happens before we've started the event loop)? |
|
And then if it is, he could try changing to and see if that helps. |
|
Sorry, I had to present in a meeting. Yeah, that f8281 has to be it. I'm building it now (windows is slow to compile) to confirm. 1c11e84 - No crash 07Aug2025 16d6695 - No crash 12Aug2025 5e965db - No crash 14Aug2025 f8281cc - This has to be it building 15d4c0a - Crash 14Aug2025 f29de89 - Crash 21Aug2025 |
|
Yes, f8281cc is when we start to crash. |
|
Lunch was good. This change if(old_view && new_view != old_view) worked on top of 1270eac |
Do i understand this correctly, this does not make dt crash or hanging? BTW while being here,
|
Correct. Changing |
|
It seems that just Could someone do more extensive testing after replacing with The thing to test, under x11/windows/mac, is if double clicking on an image in the lighttable still immediately shows the "waiting" cursor while the image is being loaded. |
|
@dterrahe : I'd like to go safe and simple for 5.4.1, so I'll fix the line as you've proposed in your first message. The change with |
|
Moving this for 5.6 now that we have a fix for 5.4.1. The goal here is to clean-up splash screen by avoid the static variables and to avoid the crawler to splash a screen even if there is no message to be displayed. |
|
@gi-man : Thanks for the hard work, please do not hesitate to test tomorrow nightly build to ensure all is back to normal. |
Possibly fixes #19992 #20111